Conversation
📝 WalkthroughWalkthroughAdds a desktop BillingProvider and billing-based gating across AI provider UIs; renames and simplifies Stripe webhook secret handling; introduces a web checkout route with validated period input; updates env defaults and dev scripts; refactors LLM/STT connection logic to support cloud models; and applies many import-order and minor edits. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopApp
participant BillingProvider
participant ReactQuery
participant Supabase
participant SettingsUI
User->>DesktopApp: Open Settings
DesktopApp->>BillingProvider: Mount (context)
BillingProvider->>ReactQuery: fetch billings(user_id)
ReactQuery->>Supabase: query billings
Supabase-->>ReactQuery: billing data
ReactQuery-->>BillingProvider: return data
BillingProvider-->>SettingsUI: provide isPro / billing state
SettingsUI->>SettingsUI: compute locked = provider.requiresPro && !isPro
alt locked
SettingsUI->>User: show PlanLockMessage, disable controls
else
SettingsUI->>User: allow provider selection/actions
end
sequenceDiagram
participant User
participant WebApp
participant CheckoutRoute
participant BillingFunction
participant Stripe
User->>WebApp: Click "Upgrade to Pro"
WebApp->>CheckoutRoute: navigate /app/checkout?period=monthly
CheckoutRoute->>BillingFunction: call createCheckoutSession({period})
BillingFunction->>Stripe: create checkout session with selected priceId
Stripe-->>BillingFunction: return session URL
BillingFunction-->>CheckoutRoute: return URL
CheckoutRoute->>User: redirect to Stripe checkout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus on:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/routes/_view/callback/auth.tsx (2)
70-84: Remove redundant redirect from useEffect.Line 72 throws a redirect inside
useEffect, which violates React's component lifecycle patterns. This can cause hydration issues and unexpected behavior. Additionally, this redirect is redundant because the web flow is already handled by thebeforeLoadhook at lines 18-27, which redirects to/appwhensearch.codeexists. ThebeforeLoadhook is the appropriate place for redirects in TanStack Router.Apply this diff to remove the redundant redirect:
useEffect(() => { - if (search.flow === "web") { - throw redirect({ to: "/app" }); - } - if ( search.flow === "desktop" && search.access_token && search.refresh_token ) { setTimeout(() => { handleDeeplink(); }, 2000); } }, [search]);
10-11: Replace token-in-URL pattern with secure OAuth flow for desktop integration.Passing access and refresh tokens as URL search parameters violates RFC 8252 (OAuth 2.0 for Native Apps). URLs are logged in browser history, sent in Referer headers, and exposed to other apps, creating information exposure vulnerabilities.
Current code (lines 40-41, 62-63) passes actual tokens via URL redirect, then extracts them for the deeplink. Instead:
- Use Authorization Code flow with PKCE; exchange the authorization code locally in the native app for tokens
- Pass only a short-lived authorization code or session identifier in the URL
- Let the native app securely exchange it for tokens using a backend endpoint
- Store tokens in platform-protected storage (OS keychain/credential vault), not URLs
This eliminates token exposure in browser history, server logs, and Referer headers.
apps/web/src/routes/webhook/stripe.ts (1)
4-78: Webhook error handler should not return 200 on verification/processing failureThe import adjustments are fine, but the POST handler currently does:
} catch (error) { console.error("[STRIPE WEBHOOK] Error processing event", error); return new Response(JSON.stringify({ received: true }), { status: 200, }); }This acknowledges the event to Stripe even when signature verification or processing fails, which can silently drop billing events (e.g., bad
STRIPE_WEBHOOK_SECRET, transient failures inprocessEvent).Consider returning a non-2xx status and an error payload instead, so Stripe can surface and/or retry failures:
- } catch (error) { - console.error("[STRIPE WEBHOOK] Error processing event", error); - return new Response(JSON.stringify({ received: true }), { - status: 200, - }); - } + } catch (error) { + console.error("[STRIPE WEBHOOK] Error processing event", error); + return new Response(JSON.stringify({ error: "Webhook error" }), { + status: 400, // or 500 if you prefer to signal server-side failure + }); + }This ensures misconfigurations or runtime issues don’t get silently acknowledged as successful deliveries.
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
96-131: Add billing to useMemo dependency array.The
useMemoat line 96 usesbilling.isProandbilling.isLoading(lines 111-116) but doesn't includebillingin the dependency array at line 129. This could cause stale availability checks when billing state changes.Apply this diff:
- }, [current_llm_provider, current_llm_model, configuredProviders]); + }, [current_llm_provider, current_llm_model, configuredProviders, billing]);
🧹 Nitpick comments (6)
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
173-200: Consider reusing shared pro‑gating helper for HyprnoteHere you hard‑code
locked = providerId === "hyprnote" && !billing.isPro, while elsewhere the PR introducesllmProviderRequiresPro. For consistency and easier future changes, consider derivinglockedfrom that helper instead of special‑casingproviderId === "hyprnote".apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
11-19: Provider typing and pro‑gating helper look solid; consider tighteningrequiresProThe new
Providertype andllmProviderRequiresProhelper are well‑structured and make gating explicit. Since every entry inPROVIDERScurrently setsrequiresPro, you could simplify the type torequiresPro: boolean(non‑optional) to avoid having to coalesce tofalseand to catch missing flags at compile time.Also applies to: 23-87, 89-91
apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
65-68: Locked STT providers use the same disabled pattern—lock message may not be visibleAs with the LLM configure view,
AccordionItemis disabled whenlockedand thePlanLockMessageis insideAccordionContent. Depending on the accordion implementation, users may not be able to open the item and therefore never see why it’s blocked.Consider:
- Leaving the item expandable while preventing edits, or
- Moving
PlanLockMessageoutside the collapsible area whenlockedis true.This would make Pro gating clearer for STT providers as well.
Also applies to: 95-105, 113-161
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
13-22: STT provider metadata and pro‑gating helper are consistent; requiresPro can likely be non‑optionalThe new
Providershape andsttProviderRequiresProhelper mirror the LLM side well and make it easy to gate STT providers by plan.As all entries in
PROVIDERSdefinerequiresPro, you could simplify the type torequiresPro: boolean(non‑optional) to ensure every future provider explicitly opts in/out of Pro gating and to avoid the need for?? false.Also applies to: 62-130, 132-134
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
106-133: Provider & model gating logic is coherent; consider deduplicatinglockedcomputation and using shared messagingThe
lockedchecks for Pro-only access on both the provider list and the model combobox are consistent and correctly gate selection based onprovider.requiresPro && !billing.isPro. However, the same computation and upgrade messaging are now implemented in two places (provider select and model select), which increases the chance of divergence later.You could:
- Centralize the “is this provider locked for this billing state?” logic in a small helper (or reuse any existing helper such as an
llmProviderRequiresPro-style utility), and- Reuse a shared “plan lock” UI component/message (if available elsewhere in the settings UI) so copy and styling stay aligned across all gated surfaces.
This would keep the Pro gating behavior DRY and easier to evolve if the rules or wording change.
Also applies to: 147-175
193-257: Avoid recomputing mapping on every billing context change; depend only onbilling.isPro
useConfiguredMappingcurrently pulls in the entirebillingcontext object as a dependency to theuseMemo, even though the mapping only depends onbilling.isPro. This means any change to loading/fetching flags will cause the mapping to recompute and all consumers to re-render, even when Pro status hasn’t changed.You can tighten the dependency to the scalar you actually use:
- const billing = useBillingAccess(); + const { isPro } = useBillingAccess(); … - if (provider.requiresPro && !billing.isPro) { + if (provider.requiresPro && !isPro) { return [provider.id, null]; } … - }, [configuredProviders, auth, billing]); + }, [configuredProviders, auth, isPro]);This keeps behavior identical while reducing unnecessary recomputation and rerenders.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (53)
.prettierrc(1 hunks)Taskfile.yaml(2 hunks)apps/api/.env.sample(1 hunks)apps/api/package.json(1 hunks)apps/api/src/env.ts(1 hunks)apps/api/src/stripe.ts(1 hunks)apps/desktop/package.json(1 hunks)apps/desktop/src/billing.tsx(1 hunks)apps/desktop/src/components/settings/account.tsx(2 hunks)apps/desktop/src/components/settings/ai/index.tsx(0 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(5 hunks)apps/desktop/src/components/settings/ai/llm/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(8 hunks)apps/desktop/src/components/settings/ai/llm/shared.tsx(8 hunks)apps/desktop/src/components/settings/ai/shared/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(7 hunks)apps/desktop/src/components/settings/ai/stt/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(5 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(7 hunks)apps/desktop/src/env.ts(1 hunks)apps/desktop/src/hooks/useBilling.ts(0 hunks)apps/desktop/src/routes/__root.tsx(2 hunks)apps/web/.env.sample(0 hunks)apps/web/package.json(1 hunks)apps/web/src/env.ts(0 hunks)apps/web/src/functions/auth.ts(1 hunks)apps/web/src/functions/billing.ts(2 hunks)apps/web/src/functions/nango.ts(1 hunks)apps/web/src/functions/stripe.ts(1 hunks)apps/web/src/functions/supabase.ts(1 hunks)apps/web/src/middleware/drizzle.ts(1 hunks)apps/web/src/middleware/nango.ts(1 hunks)apps/web/src/middleware/supabase.ts(1 hunks)apps/web/src/routes/__root.tsx(1 hunks)apps/web/src/routes/_view/app/account.tsx(3 hunks)apps/web/src/routes/_view/app/checkout.tsx(1 hunks)apps/web/src/routes/_view/blog/$slug.tsx(1 hunks)apps/web/src/routes/_view/callback/auth.tsx(1 hunks)apps/web/src/routes/_view/docs/-components.tsx(1 hunks)apps/web/src/routes/_view/download/index.tsx(1 hunks)apps/web/src/routes/_view/enterprise.tsx(1 hunks)apps/web/src/routes/_view/index.tsx(1 hunks)apps/web/src/routes/_view/pricing.tsx(1 hunks)apps/web/src/routes/_view/product/ai-assistant.tsx(1 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(1 hunks)apps/web/src/routes/_view/product/api.tsx(1 hunks)apps/web/src/routes/_view/product/mini-apps.tsx(1 hunks)apps/web/src/routes/_view/product/notepad.tsx(1 hunks)apps/web/src/routes/_view/product/workflows.tsx(1 hunks)apps/web/src/routes/_view/route.tsx(1 hunks)apps/web/src/routes/_view/templates.tsx(1 hunks)apps/web/src/routes/auth.tsx(1 hunks)apps/web/src/routes/webhook/stripe.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/web/src/env.ts
- apps/desktop/src/hooks/useBilling.ts
- apps/desktop/src/components/settings/ai/index.tsx
- apps/web/.env.sample
🧰 Additional context used
🧬 Code graph analysis (14)
apps/web/src/routes/_view/app/checkout.tsx (2)
apps/web/src/routes/_view/app/account.tsx (1)
Route(11-14)apps/web/src/functions/billing.ts (1)
createCheckoutSession(12-66)
apps/desktop/src/components/settings/ai/stt/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttProviderRequiresPro(132-134)
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/llm/configure.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/shared/index.tsx (3)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)
apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
apps/desktop/src/components/settings/ai/stt/shared.tsx (2)
ProviderId(24-24)PROVIDERS(62-130)
apps/web/src/functions/billing.ts (2)
crates/aec/src/lib.rs (1)
data(51-53)apps/api/src/stripe.ts (1)
stripe(6-8)
apps/desktop/src/billing.tsx (1)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)
apps/desktop/src/components/settings/account.tsx (3)
apps/desktop/src/env.ts (1)
env(4-14)apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/billing.tsx (2)
useBillingAccess(108-116)BillingAccess(31-31)
apps/desktop/src/components/settings/ai/llm/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
llmProviderRequiresPro(89-91)
apps/desktop/src/routes/__root.tsx (1)
apps/desktop/src/billing.tsx (1)
BillingProvider(35-106)
apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(108-116)apps/desktop/src/components/settings/ai/shared/index.tsx (3)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
apps/desktop/src/components/settings/ai/llm/shared.tsx (2)
ProviderId(21-21)PROVIDERS(23-87)
apps/api/src/stripe.ts (1)
apps/api/src/env.ts (1)
env(4-20)
🪛 dotenv-linter (4.0.0)
apps/api/.env.sample
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (43)
apps/web/src/routes/_view/templates.tsx (1)
8-10: Import reorganization aligns with Prettier config updates.The DownloadButton and SlashSeparator imports follow the new @/ alias grouping pattern, properly separated from external packages. Both components are used in the component (lines 141, 176, and 189), so no unused imports. The change is well-organized and consistent with the PR's import cleanup work across web routes.
apps/web/src/routes/_view/download/index.tsx (1)
6-7: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/routes/_view/blog/$slug.tsx (1)
9-10: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/functions/nango.ts (1)
4-4: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/desktop/package.json (1)
10-13: LGTM! Improved developer experience.Adding the
--silentflag toenv-cmdreduces log noise during development and build processes.apps/web/src/routes/_view/pricing.tsx (1)
6-7: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/routes/_view/enterprise.tsx (1)
6-6: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/web/src/middleware/nango.ts (1)
4-4: LGTM! Import ordering standardization.The import reordering aligns with the project's formatting standards and has no functional impact.
apps/api/.env.sample (1)
2-2: Environment variable rename verified as complete and consistent.Verification confirms the rename from
STRIPE_WEBHOOK_SIGNING_SECRETtoSTRIPE_WEBHOOK_SECREThas been applied uniformly throughout the codebase:
- No remnants of the old variable name remain
- New variable is properly defined in both web and API environment schemas (env.ts)
- Actual usage correctly updated in Stripe integration files
- No deployment, CI/CD, or Docker configuration files reference the old variable
- Environment generation (Taskfile.yaml) uses the new variable name
All concerns raised in the review have been addressed.
apps/api/src/stripe.ts (2)
34-34: Good addition for debugging.Adding error logging improves troubleshooting webhook verification failures. This complements the existing error message extraction on line 35.
26-26: Code change is correct and consistent.Verification confirms the rename from
STRIPE_WEBHOOK_SIGNING_SECRETtoSTRIPE_WEBHOOK_SECREThas been applied consistently across all files. No stale references to the old variable name remain, and all usages (environment schemas in both apps, webhook handlers, and task configuration) properly reference the new variable name.apps/api/package.json (1)
6-6: LGTM! Reduced verbosity during development.The
--silentflag suppresses env-cmd output, reducing console noise. This aligns with similar changes across other packages in the PR..prettierrc (1)
9-9: LGTM! Import order configuration aligned with codebase aliases.The new
@/import path pattern groups these imports logically between@hyprpackages and relative imports, supporting the import standardization evident throughout this PR.apps/web/src/routes/_view/docs/-components.tsx (1)
18-19: LGTM! Import reordering aligns with Prettier configuration.The
CtaCardimport has been repositioned to follow the new@/import grouping pattern. No functional changes.apps/web/package.json (1)
6-6: LGTM! Consistent silent flag usage across packages.Adding
--silenttoenv-cmdreduces development console noise. This change is consistent with the same pattern applied to other packages in this PR.apps/web/src/routes/_view/index.tsx (1)
1-9: LGTM! Import reorganization consistent with Prettier configuration.Imports have been reordered to align with the new
@/import grouping rules. No functional changes.apps/desktop/src/env.ts (1)
7-7: LGTM! Default URL improves local development experience.Changing from
.optional()to.default("http://localhost:3000")provides a sensible fallback for local development and eliminates the possibility of undefined values.Ensure that production and staging environments explicitly set
VITE_APP_URLto their respective domains, as the localhost default should only be used during local development.apps/web/src/functions/billing.ts (2)
8-10: LGTM! Input validation ensures type safety.The zod schema properly constrains the period to only valid values ("monthly" or "yearly"), preventing invalid inputs from reaching the handler.
47-50: Environment variables are properly defined and validated.Verification confirms both
STRIPE_MONTHLY_PRICE_IDandSTRIPE_YEARLY_PRICE_IDare defined in the environment schema (apps/web/src/env.ts, lines 14-15) with proper validation requiring non-empty strings. The billing.ts implementation correctly uses these variables.Taskfile.yaml (2)
247-254: LGTM! Simplified Stripe webhook secret handling.The new approach directly retrieves the secret using
stripe listen --print-secretand includes proper validation to ensure the secret is non-empty before writing. This is cleaner than the previous FIFO-based extraction.
241-241: LGTM! Streamlined cleanup and webhook listener invocation.The cleanup now only removes the generated env file (no FIFO cleanup needed), and the webhook listener starts directly after the secret is written. This simplifies the flow and reduces complexity.
Also applies to: 309-311
apps/api/src/env.ts (1)
14-14: All environment file references have been successfully updated.The verification confirms the Stripe webhook secret rename is complete with no issues:
- Zero remaining references to
STRIPE_WEBHOOK_SIGNING_SECRET- All new
STRIPE_WEBHOOK_SECRETreferences correctly placed in.env.samplefiles, environment schemas (apps/api/src/env.ts,apps/web/src/env.ts), and webhook implementations (apps/api/src/stripe.ts,apps/web/src/routes/webhook/stripe.ts)- Both
apps/apiandapps/webconsistently updatedapps/web/src/routes/_view/product/ai-notetaking.tsx (1)
7-8: MockWindow & SlashSeparator imports match usageBoth components are used later in the file and the
@/components/...paths look consistent with the rest of the codebase. No behavioral changes introduced here.apps/web/src/routes/_view/product/workflows.tsx (1)
6-6: SharedImageimport is consistent with usageThe new
Imageimport aligns with how this component renders icons throughout the file and keeps image behavior consistent. No issues from this change.apps/web/src/routes/_view/product/ai-assistant.tsx (1)
6-6: SlashSeparator import aligns with component usage
SlashSeparatoris used multiple times in this route; importing it from@/components/slash-separatoris correct and purely structural.apps/web/src/functions/stripe.ts (1)
4-9: Server-only Stripe client factory looks correctUsing
createServerOnlyFnaround the Stripe client instantiation keepsenv.STRIPE_SECRET_KEYserver-only while providing a simplegetStripeClient()helper for server code. The explicitapiVersionis also good for stability.apps/web/src/routes/_view/product/notepad.tsx (1)
6-6: SlashSeparator import is consistent and necessary
SlashSeparatoris used multiple times in this component; importing it from@/components/slash-separatoris correct and keeps imports consistent with other routes.apps/web/src/routes/_view/product/api.tsx (1)
5-5: MockWindow import matches the hero preview usageThe
MockWindowimport from@/components/mock-windowcorrectly backs the terminal-style preview in the hero. No behavioral changes beyond wiring the existing component.apps/web/src/routes/__root.tsx (1)
9-12: Alias imports for core root wiring look goodImporting
NotFoundDocument,fetchUser, andappCssvia the@/aliases matches how they’re used in the route config (beforeLoad,notFoundComponent, andlinks). No behavioral change from these import adjustments.apps/web/src/routes/_view/product/mini-apps.tsx (1)
6-7: LGTM! Clean addition of visual separator.The SlashSeparator import and its usage throughout the component provide consistent section separation in the mini-apps layout.
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
124-130: LGTM! Shared component for Pro-gated features.The PlanLockMessage component provides a consistent UI pattern for displaying upgrade prompts across AI provider settings. The amber color scheme appropriately conveys a locked/gated state.
apps/desktop/src/routes/__root.tsx (1)
29-34: LGTM! Proper provider nesting order.The BillingProvider is correctly positioned between AuthProvider and the application content, ensuring billing context depends on authentication state and is available to all child components including devtools.
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
111-118: LGTM! Proper Pro-gating for LLM providers.The billing-based access control correctly blocks providers that require Pro, with a loading-aware message that provides good user feedback.
apps/desktop/src/components/settings/ai/stt/health.tsx (1)
92-99: LGTM! Consistent Pro-gating pattern for STT providers.The billing-based access control follows the same pattern as LLM providers, with appropriate loading state handling and clear upgrade messaging.
apps/web/src/routes/_view/app/account.tsx (2)
73-80: LGTM! Cleaner checkout flow for free users.The migration from mutation-based upgrades to a dedicated checkout route simplifies the account page logic and centralizes billing flow.
98-104: Trial functionality remains active—review comment misidentified the change scope.The lines 98-104 modify only the "free" plan button, not trial functionality. The component maintains distinct branches for four plan states: "free", "trial", "trial_over", and "pro". Trial users continue to see "Manage Billing" (lines 83–92), while only free-tier users see "Upgrade to Pro" linking to checkout. Stripe webhooks still handle trial events ("customer.subscription.trial_will_end"), and the FAQ continues mentioning the 14-day free trial. The button change reflects a free-to-paid upgrade path, not trial removal.
Likely an incorrect or invalid review comment.
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
102-132: LGTM! Clear Pro-gating UI with excellent user feedback.The provider selection properly gates Pro-required providers with:
- Visual Pro badge for identification
- Disabled state when locked
- Clear "Upgrade to Pro" messaging
The implementation provides excellent UX for gated features.
230-232: LGTM! Correct data-level gating for Pro providers.The useConfiguredMapping hook properly filters out Pro-required providers when the user doesn't have Pro access, ensuring the data layer enforces the same constraints as the UI.
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
50-52: Locked providers are disabled but the lock message may never be visible
AccordionItemis markeddisabled={locked}, and the lock message (PlanLockMessage) lives insideAccordionContent. If the accordion implementation prevents opening disabled items, users will only see a dimmed, non‑clickable card without any explanation.You may want to:
- Keep the trigger enabled but block configuration actions, or
- Render the
PlanLockMessageoutside the collapsible content so it’s always visible whenlockedis true.This will make the Pro gating clearer without relying on accordion behavior.
Also applies to: 90-101, 107-158
apps/desktop/src/components/settings/ai/stt/configure.tsx (1)
242-261: HyprProviderCloudRow text gating is clear and non‑breakingUsing
billing.isProsolely to switch between “For Pro Users” and “Coming soon” while keeping the button disabled is a safe way to surface future Pro‑only functionality without introducing new flows yet. This looks good and should be easy to extend once the feature is live.apps/desktop/src/components/settings/account.tsx (2)
13-53: Auth and billing wiring for SettingsAccount looks correctUsing
useAuthanduseBillingAccessto gate the whole view, with a clear unauthenticated sign‑in card and authenticated account/billing containers, is a clean refactor. The URL construction viaWEB_APP_BASE_URLand the callbacks for account/checkout navigation are straightforward and easy to reason about.
259-277: PLANS andformatSubscriptionStatusutilities are simple and readableThe
PLANSmap andformatSubscriptionStatushelper nicely encapsulate display details away from the main component. This should make future plan tweaks or status formatting changes easy to implement without touching the billing logic.apps/desktop/src/billing.tsx (1)
118-136:computeIsPrologic matches typical Stripe subscription modelingThe
computeIsProhelper correctly checks both subscription status ("active"or"trialing") and whether any subscription item’s product matchesenv.VITE_PRO_PRODUCT_ID, handling both string IDs and expanded product objects. This is a solid, explicit definition of “Pro” and looks good as a central place to evolve the rules if needed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/components/settings/ai/llm/health.tsx (1)
96-122: Add billing dependencies to the availability memo.
useAvailabilityreadsbilling.isProandbilling.isLoading, but the memo only depends on provider/model. React will keep returning a stale result after billing state changes (e.g., upgrading to Pro still shows the upgrade gating until you change providers). Add the billing flags to the dependency array so the memo re-renders when access changes.- }, [current_llm_provider, current_llm_model, configuredProviders]); + }, [ + current_llm_provider, + current_llm_model, + configuredProviders, + billing.isPro, + billing.isLoading, + ]);apps/desktop/src/components/settings/ai/stt/health.tsx (1)
20-50: Cloud detection likely too broad; hyprnote non‑cloud models skip local health handling
isCloudis currentlycurrent_stt_provider === "hyprnote" || current_stt_model === "cloud". That means any hyprnote model (including localam-*/Quantized*) is treated as “cloud”, souseConnectionHealthreturns based only onconnand never inspects the local STT server status.In
apps/desktop/src/config/registry.ts, the STT side effects still start the local server whenprovider === "hyprnote"andmodelis not"cloud"and matches the local prefixes, which implies those combinations are still local-mode.To keep health in sync with the registry behavior,
isCloudshould probably only be true for the explicit"cloud"model (and optionally only when paired with the hyprnote provider), so local hyprnote models continue to use thelocalstatus path.A minimal fix:
- const isCloud = - current_stt_provider === "hyprnote" || current_stt_model === "cloud"; + const isCloud = + current_stt_provider === "hyprnote" && current_stt_model === "cloud";This keeps cloud-only handling for the new cloud mode while preserving the existing local health behavior for non‑cloud hyprnote models.
♻️ Duplicate comments (1)
apps/desktop/src/billing.tsx (1)
43-86: Cached billing data persists after logout—privacy & gating issueThe issue flagged in the previous review remains unaddressed. When the user logs out,
enabledbecomesfalse, but React Query retains the last successfulqueryDatain cache. Line 85 assignsdata = queryData ?? nullwithout gating on auth state, soisProcontinues to reflect the previous user's subscription even afterauth.sessionis cleared.Apply this diff to explicitly null out data when unauthenticated:
- const data = queryData ?? null; + const hasUser = !!auth?.session?.user?.id; + const data = hasUser ? queryData ?? null : null;Also update the context value (line 94) to gate
isPro:- isPro: !!data?.isPro, + isPro: hasUser && !!data?.isPro,And add
auth?.session?.user?.idto theuseMemodependency array (lines 104-114) so the context recomputes on auth changes.
🧹 Nitpick comments (6)
apps/desktop/src/components/settings/ai/stt/health.tsx (1)
62-103: Billing gating for STT availability looks correct; consider softening behavior while loadingThe
sttProviderRequiresPro+billing.isProgate is wired correctly and short‑circuits availability with clear messages, including a special"Checking plan access..."string whilebilling.isLoadingis true.One trade‑off: while loading, Pro‑required providers are effectively locked even for Pro users until
isProflips true. If that brief lock is undesirable, you could gate on something like!billing.isLoading && !billing.isProinstead, or expose a separateisReadyflag from billing.apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
51-59: Locked providers usedisabledAccordionItems; verify content is still reachableBoth
NonHyprProviderCardandHyprProviderCardcompute alockedflag and passdisabled={locked}to<AccordionItem>, while the locked UI (PlanLockMessage / upgrade messaging) lives inside<AccordionContent>.Depending on how
@hypr/ui’s Accordion handlesdisabled, this may prevent users from expanding a locked item at all, meaning the PlanLockMessage and upgrade affordance are never seen. The trigger is visually dimmed, but there might be no way to discover why the provider is unavailable.A safer pattern is often to leave the item expandable and only gate the interactive form:
- Keep the
cursor-not-allowed opacity-30styling on the trigger to convey lock.- Remove
disabled={locked}fromAccordionItem.- Continue to render
PlanLockMessageinstead of the form (or in addition, for Hyprnote), as you already do.Also applies to: 91-162, 174-205
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
188-257: useConfiguredMapping cleanly centralizes provider availability (auth + billing + config)The mapping function correctly short‑circuits to
nullin all cases where model listing should be disabled:
- Pro‑required providers when
!billing.isPro.- Hyprnote when there is no
auth?.session.- Any provider missing
base_urlor requiredapi_key.Otherwise, it builds an appropriate
listModelsFuncper provider.If you find the Pro‑lock condition creeping into more places, you could consider extracting a small helper (e.g.,
isProviderLocked(provider, billing)) to reuse between here and the UI components, but it’s not strictly necessary given current usage.apps/desktop/src/config/registry.ts (1)
90-124: Local STT start condition matches new “cloud” model semantics; consider de‑duplicating logicThe updated side effects for
current_stt_providerandcurrent_stt_model:
- Safely treat both values as
string | undefined.- Only call
localSttCommands.startServerwhen:
- provider is
"hyprnote",- model is set,
- model is not
"cloud", and- model starts with
"am-"or"Quantized".That aligns well with the new “cloud” model and prevents accidental
startServercalls for unsupported strings.The conditional block is duplicated in both sideEffect handlers, though. If you find these conditions evolving (e.g., more local models), consider extracting a small helper like
maybeStartLocalSttForConfig(provider, model)to keep them in sync and reduce maintenance overhead.apps/desktop/src/billing.tsx (2)
87-89: Remove unusedauthdependency from upgradeToPro callbackThe
authvariable is listed in the dependency array but is never used within theupgradeToProcallback body. This causes unnecessary re-creation of the callback wheneverauthchanges.Apply this diff:
const upgradeToPro = useCallback(() => { openUrl(`${env.VITE_APP_URL}/app/checkout?period=monthly`); - }, [auth]); + }, []);
132-150: Guard against undefinedVITE_PRO_PRODUCT_IDfor clearer failure modeThe function relies on
env.VITE_PRO_PRODUCT_IDto determine Pro status, but that environment variable is optional (perenv.ts). If it's undefined, the product comparison (lines 145-146) will always fail silently, causing all users to appear non-Pro.Adding an explicit guard makes the failure mode clear and prevents subtle bugs:
function computeIsPro( subscription: Stripe.Subscription | null | undefined, ): boolean { if (!subscription) { return false; } + + if (!env.VITE_PRO_PRODUCT_ID) { + console.warn('VITE_PRO_PRODUCT_ID is not configured; Pro status unavailable'); + return false; + } const hasValidStatus = ["active", "trialing"].includes(subscription.status);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/desktop_cd.yaml(1 hunks)apps/api/src/billing.ts(1 hunks)apps/api/src/deepgram.ts(1 hunks)apps/desktop/src/billing.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(5 hunks)apps/desktop/src/components/settings/ai/llm/health.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(9 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(7 hunks)apps/desktop/src/components/settings/ai/stt/health.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(6 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(8 hunks)apps/desktop/src/config/registry.ts(2 hunks)apps/desktop/src/env.ts(1 hunks)apps/desktop/src/hooks/useLLMConnection.ts(1 hunks)apps/desktop/src/hooks/useSTTConnection.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/components/settings/ai/stt/configure.tsx
- apps/desktop/src/components/settings/ai/stt/select.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/llm/configure.tsx (3)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/shared/index.tsx (4)
useProvider(57-69)PlanLockMessage(124-130)FormField(71-122)StyledStreamdown(40-55)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
apps/desktop/src/components/settings/ai/llm/shared.tsx (2)
ProviderId(21-21)PROVIDERS(23-87)
apps/desktop/src/components/settings/ai/stt/health.tsx (3)
apps/desktop/src/config/use-config.ts (1)
useConfigValues(42-72)apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttProviderRequiresPro(137-139)
apps/desktop/src/components/settings/ai/llm/health.tsx (2)
apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
llmProviderRequiresPro(89-91)
apps/desktop/src/hooks/useLLMConnection.ts (1)
apps/desktop/src/env.ts (1)
env(4-15)
apps/desktop/src/billing.tsx (2)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/env.ts (1)
env(4-15)
apps/desktop/src/hooks/useSTTConnection.ts (2)
apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/env.ts (1)
env(4-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (7)
apps/api/src/deepgram.ts (1)
367-369: Good fix: usingsetprevents duplicate parameters.The change from
appendtosetensures that duplicate query parameter keys are replaced rather than accumulated, which is the correct behavior for most API parameters.apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
207-237: ProviderContext Pro‑aware messaging for Hyprnote reads cleanlyThe updated
ProviderContextcleanly adds billing‑aware messaging:
- Uses
useBillingAccessto pullisProandupgradeToPro.- For hyprnote when not Pro, wraps the standard
StyledStreamdowncontent with an inline “Upgrade to Pro” button.- Falls back to the previous content rendering for all other providers or when already Pro.
This keeps the contextual help strings centralized and provides a clear CTA without complicating the calling components.
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
30-186: Provider/model selection Pro gating looks consistent and ergonomicWithin
SelectProviderAndModel:
- The provider field listener that sets model to
"Auto"for"hyprnote"and clears it otherwise keeps the pair in a valid state.- Provider
SelectItems are correctly disabled when either not configured (!configuredProviders[provider.id]) or Pro‑locked (provider.requiresPro && !billing.isPro), and show succinct upgrade messaging when locked.- The model field reuses the same lock condition and prevents listing or selecting models when the provider is unavailable, while adding a clear “Upgrade to Pro to pick … models.” hint.
Behavior matches the PROVIDERS metadata and the Pro expectations from
configure.tsx, and the UX should be predictable.apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
13-22: STT Provider metadata and Pro‑gating helper are well‑structured; note gating is currently a no‑opThe new
Providertype,PROVIDERSdefinition,"cloud"model, andsttProviderRequiresProhelper are all wired coherently:
satisfies readonly Provider[]locks in the provider shape and improves type‑safety.displayModelIdhandling of"cloud"→"Cloud"keeps the UI readable.sttProviderRequiresProcleanly abstracts therequiresProflag with a safe default.With the current data, every STT provider has
requiresPro: false, so the billing gate instt/health.tsxwill never trigger yet. That’s fine if STT should remain free for now; just be aware that flipping any provider torequiresPro: truelater will immediately activate the gate in the health/availability flow.Also applies to: 38-41, 66-135, 137-139
apps/desktop/src/billing.tsx (3)
1-39: LGTM!The imports and type definitions are clean and well-structured. The type hierarchy (BillingRow → BillingData → BillingContextValue) clearly models the data flow from database through enrichment to context API.
122-130: LGTM!The
useBillingAccesshook follows the standard context consumer pattern and matches the style used inuseAuth. Clear error messaging ensures developers are alerted if the hook is used outside the provider.
132-150: No compatibility issues found—Stripe types are correctThe Stripe SDK version installed is ^19.3.0, and subscription.status values "active" and "trialing" are both valid in Stripe v19. The subscription structure being accessed (items.data, price.product) aligns with the standard Stripe subscription object. The code is compatible with the installed Stripe version.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/settings/ai/stt/health.tsx (1)
26-34: Fix inconsistent cloud detection logic in health.tsx.The
isCloudcheck uses||(OR) but should use&&(AND) to match the logic inuseSTTConnection.ts(line 33). The "cloud" model is only defined for the "hyprnote" provider, so both conditions must be true. Using OR creates unintended behavior if data state allows a non-hyprnote provider with model="cloud".// Current (line 26-27): const isCloud = current_stt_provider === "hyprnote" || current_stt_model === "cloud"; // Should be: const isCloud = current_stt_provider === "hyprnote" && current_stt_model === "cloud";This ensures consistency with
useSTTConnection.tsand makes the intent explicit: cloud mode is only active when both provider is hyprnote AND model is cloud.
🧹 Nitpick comments (3)
apps/desktop/src/hooks/useLLMConnection.ts (2)
21-44: Consider exporting the connection types for wider reuseThe
LLMConnectionInfo/LLMConnectionStatus/LLMConnectionResultshapes are well-designed and make downstream handling much clearer. SinceuseLLMConnectionis exported and its return type is part of the public surface, it’s worth explicitlyexport‑ing at leastLLMConnectionStatusandLLMConnectionResultso other modules can type against them and (in some TS configs) avoid “exported function uses private type” diagnostics.
121-229: Validate provider before casting and reportingmissing_modelThe overall flow and status modeling in
useLLMConnectionare strong, but there’s a subtle ordering issue:
- You cast
current_llm_providertoProviderIdand may return amissing_modelstatus before checking whether that provider actually exists inPROVIDERS.- If the store ever contains an unknown provider ID (migration, typo, stale data), you’ll report
missing_modelinstead of the more accurateprovider_not_found.You can make this more robust and type‑safe by resolving the provider definition first and deriving the typed
providerIdfrom it, then checking for the model, e.g.:- const providerConfig = main.UI.useRow( - "ai_providers", - current_llm_provider ?? "", - main.STORE_ID, - ) as main.AIProviderStorage | undefined; + const providerConfig = main.UI.useRow( + "ai_providers", + current_llm_provider ?? "", + main.STORE_ID, + ) as main.AIProviderStorage | undefined; return useMemo<LLMConnectionResult>(() => { if (!current_llm_provider) { return { conn: null, status: { status: "pending", reason: "missing_provider" }, }; } - const providerId = current_llm_provider as ProviderId; - - if (!current_llm_model) { - return { - conn: null, - status: { - status: "pending", - reason: "missing_model", - providerId, - }, - }; - } - - const providerDefinition = PROVIDERS.find( - (provider) => provider.id === current_llm_provider, - ); + const providerDefinition = PROVIDERS.find( + (provider) => provider.id === current_llm_provider, + ); if (!providerDefinition) { return { conn: null, status: { status: "error", reason: "provider_not_found", providerId: current_llm_provider, }, }; } + const providerId = providerDefinition.id; + + if (!current_llm_model) { + return { + conn: null, + status: { + status: "pending", + reason: "missing_model", + providerId, + }, + }; + }This keeps runtime behavior sane for unexpected provider IDs and gives you a properly typed
providerIdwithout an unsafe cast.apps/desktop/src/components/settings/account.tsx (1)
12-12: Redundant fallback; the env schema already defaults VITE_APP_URL.The
?? "http://localhost:3000"is unnecessary sinceenv.VITE_APP_URLhas.default("http://localhost:3000")in the schema and will never be undefined.Apply this diff to simplify:
-const WEB_APP_BASE_URL = env.VITE_APP_URL ?? "http://localhost:3000"; +const WEB_APP_BASE_URL = env.VITE_APP_URL;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/settings/account.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/health.tsx(4 hunks)apps/desktop/src/hooks/useLLMConnection.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/desktop/src/components/settings/ai/stt/health.tsx (3)
apps/desktop/src/config/use-config.ts (1)
useConfigValues(42-72)apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttProviderRequiresPro(137-139)
apps/desktop/src/components/settings/account.tsx (3)
apps/desktop/src/env.ts (1)
env(4-15)apps/desktop/src/auth.tsx (1)
useAuth(184-192)apps/desktop/src/billing.tsx (1)
useBillingAccess(122-130)
apps/desktop/src/hooks/useLLMConnection.ts (2)
apps/desktop/src/components/settings/ai/llm/shared.tsx (2)
ProviderId(21-21)PROVIDERS(23-87)apps/desktop/src/env.ts (1)
env(4-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (9)
apps/desktop/src/components/settings/ai/stt/health.tsx (2)
3-3: LGTM! Imports are properly structured.The new imports for billing access and provider requirements are correctly added and will support the gating logic below.
Also applies to: 7-12
91-98: Billing context properties properly typed and implemented.Verification confirms that
billing.isProandbilling.isLoadingare correctly defined asbooleanproperties in theBillingContextValuetype, and theBillingProviderproperly hydrates these values. The code correctly accesses these typed properties.apps/desktop/src/hooks/useLLMConnection.ts (1)
46-119: Centralizing provider wiring onconnlooks solidThe refactor to have
useLanguageModeldepend only onconn(and returnnullwhen it’s absent) is clean and keeps all provider/env/auth logic insideuseLLMConnection. Provider‑specific branches forhyprnote,anthropic,openrouter,openai, and the generic OpenAI‑compatible case are consistent and correctly feedmodelIdthrough to the SDK factories, withwrapWithThinkingMiddlewareapplied uniformly.apps/desktop/src/components/settings/account.tsx (6)
1-10: LGTM!Imports are clean and appropriate for the new billing-aware account settings UI.
14-26: LGTM!Proper hook usage and callback memoization.
28-40: LGTM!Clear authentication gating with appropriate sign-in prompt.
42-84: LGTM! Simplified billing display resolves previous edge case.The new implementation takes a straightforward approach: showing subscription details only when
stripe_subscriptionexists and the Manage button only whenhasStripeCustomeris true. This elegantly avoids the "pro without Stripe" edge case flagged in the previous review.
99-107: LGTM!Proper Unix timestamp conversion (seconds to milliseconds) and clean date formatting.
109-132: LGTM!Clean, reusable presentational component with good prop flexibility.
1 participant
No description provided.